Skip to content

Refactor: remove legacy graph output metadata - #1439

Merged
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
zmnobug:refactor/remove-legacy-graph-output-metadata
Jul 22, 2026
Merged

Refactor: remove legacy graph output metadata#1439
ChaoZheng109 merged 1 commit into
hw-native-sys:mainfrom
zmnobug:refactor/remove-legacy-graph-output-metadata

Conversation

@zmnobug

@zmnobug zmnobug commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove unused graph_output_ptr and graph_output_size metadata, initialization, documentation, and obsolete test scaffolding
  • Pin shared-memory header sizes and key offsets with compile-time assertions so ABI layout drift fails during the build
  • Simplify failed-execution copy-back warnings to avoid repeating error codes already reported centrally

Testing

  • Full editable runtime rebuild for A2/A3 and A5, simulation and onboard variants
  • C++ unit tests: 60/60 passed
  • A2/A3 and A5 simulation smoke tests for both runtime implementations
  • Onboard runtime smoke tests not run locally because device architecture detection was unavailable

Follow-up to #1412.

zm
Refactor: remove legacy graph output metadata
7985f2d 
- Delete unused packed-output fields and their initialization
- Assert shared-memory sizes and key offsets to catch ABI drift
- Avoid repeating centrally reported execution codes in copy-back logs
@gemini-code-assist

gemini-code-assist Bot commented Jul 22, 2026

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 45b11898-cd07-4f10-9f62-be149739d297

📥 Commits

Reviewing files that changed from the base of the PR and between 56919a7 and 7985f2d.

📒 Files selected for processing (14)
  • src/a2a3/runtime/host_build_graph/docs/RUNTIME_LOGIC.md
  • src/a2a3/runtime/host_build_graph/host/runtime_maker.cpp
  • src/a2a3/runtime/host_build_graph/runtime/pto_shared_memory.h
  • src/a2a3/runtime/host_build_graph/runtime/shared/pto_shared_memory.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md
  • src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.h
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cpp
  • src/a5/runtime/host_build_graph/host/runtime_maker.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md
  • src/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.h
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cpp
  • tests/ut/cpp/common/test_trb_runtime_temp_buffer.cpp
💤 Files with no reviewable changes (7)
  • src/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cpp
  • tests/ut/cpp/common/test_trb_runtime_temp_buffer.cpp
  • src/a2a3/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md
  • src/a2a3/runtime/host_build_graph/docs/RUNTIME_LOGIC.md
  • src/a2a3/runtime/host_build_graph/runtime/shared/pto_shared_memory.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cpp
  • src/a5/runtime/tensormap_and_ringbuffer/docs/RUNTIME_LOGIC.md
📝 Walkthrough

Walkthrough

The change removes legacy packed-output metadata from shared-memory headers and initialization paths, adds exact compile-time layout checks, updates documentation and a unit test, and simplifies execution-failure warning messages across runtime variants.

Changes

Shared-memory contract cleanup

Layer / File(s) Summary
Shared-memory layout contract
src/a2a3/runtime/.../pto_shared_memory.h, src/a5/runtime/.../pto_shared_memory.h		 Removes graph_output_ptr and graph_output_size, includes <stddef.h>, and adds exact size and offset assertions for shared-memory structures.
Initialization and contract consumers
src/*/runtime/.../pto_shared_memory.cpp, src/*/docs/RUNTIME_LOGIC.md, tests/ut/cpp/common/test_trb_runtime_temp_buffer.cpp		 Stops initializing and testing the removed fields, and removes them from runtime layout documentation.
Execution-failure warning text
src/*/host/runtime_maker.cpp		 Removes execution_rc from warnings emitted when tensor copy-back is skipped after execution failure.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Poem

A rabbit pruned two fields from the ring,
Checked every offset and size of the thing.
Old packed-output notes hopped away,
While warnings grew shorter today.
“Layout is snug!” the bunny sings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: removing legacy graph output metadata.
Description check ✅ Passed The description matches the changeset, covering metadata removal, layout assertions, and log simplification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ChaoZheng109
ChaoZheng109 merged commit e60cfac into hw-native-sys:main Jul 22, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChaoZheng109 ChaoZheng109 ChaoZheng109 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zmnobug @ChaoZheng109